perf: avoid allocating a mapping lambda on every labelValues() call - #2442
Open
david-mollitor-db wants to merge 2 commits into
Open
david-mollitor-db wants to merge 2 commits into
david-mollitor-db wants to merge 2 commits into
Conversation
david-mollitor-db
requested review from
dhoard,
fstab,
jaydeluca and
zeitlinger
as code owners
September 1, 2026 19:57
labelValues() went straight to data.computeIfAbsent(key, l -> ...). The mapping function captures 'this', so a new lambda instance was allocated on every call - including the common case where the data point already exists, since the lambda argument is constructed before computeIfAbsent runs. Add a data.get(key) fast path that returns the existing data point without constructing the lambda. In a JMH benchmark of a histogram-heavy workload this cut record-path allocation by ~18% (exactly the 16-byte captured lambda per observation). On the miss path, validate the label values on the raw array before computeIfAbsent, so the null check runs outside the ConcurrentHashMap bin lock and without List indirection. Creation stays inside computeIfAbsent: newDataPoint() has side effects (a native histogram may schedule a reset task), so at-most-once creation must be preserved. Behavior is unchanged (verified by the core tests, including StatefulMetricTest). Signed-off-by: David Mollitor <david.mollitor@databricks.com>
david-mollitor-db
force-pushed
the
labelvalues-fast-path
branch
from
September 14, 2026 13:46
bf05f3b to
6bcd096
Compare
Member
|
Pending the benchmark-only PR #2468 That PR adds repeated existing-label lookup/increment benchmarks with cached-data-point baselines, Once #2468 is merged, update this branch and retrigger the benchmark label so both base and head |
Contributor
Benchmark resultsBenchmark run succeeded for
Prometheus Java Client BenchmarksRun Information
Comparison with base
Results for PR headCounterBenchmark
HistogramBenchmark
HistogramTextFormatBenchmark
TextFormatUtilBenchmark
Allocation per operationJMH GC profiler
Raw ResultsNotes
Benchmark Descriptions
|
Member
|
I ran a matched local JMH comparison using the identical #2468 lookup/cached harness on both candidate base and head, with #2471 applied to both. Configuration: JDK 25.0.3, 3 forks, 3x10s warmup, 5x10s measurement, GC profiler. Results (base -> head): lookup single-thread 62.85M -> 88.12M ops/s (+40.2%), 64 -> 48 B/op (-25%); lookup 4-thread 225.36M -> 295.40M ops/s (+31.1%), 64 -> 48 B/op (-25%). Cached controls: 520.25M -> 534.07M (+2.7%) single-thread and 1.504B -> 1.513B (+0.6%) 4-thread, both ~0 B/op. This supports #2442 as a worthwhile release optimization. Full raw JSON is available locally during review. |
zeitlinger
added a commit
that referenced
this pull request
Sep 15, 2026
…2468) ## Summary Benchmark-only follow-up for the release review; no production metric changes. - Restrict both base and head PR benchmark runs to client_java counter/histogram methods. OpenTelemetry, Codahale, and legacy simpleclient comparisons remain in full/local/nightly runs. Keep all client_java exposition benchmarks, including OpenMetrics. - Add repeated existing-label lookup + increment benchmarks and cached-data-point baselines, with one-thread and four-thread variants. One invocation is one metric update. - Report GC profiler allocation in B/op separately from throughput, including descriptive base/head allocation deltas only when configurations match. - Add selection/report regression tests and run benchmark tooling tests in lint CI. - Document operation units and the new benchmark workflow. ## Related PR [Label lookup optimization #2442](#2442) is pending this benchmark infrastructure. After this lands, update that branch and rerun the benchmarks so base and head contain identical lookup benchmark code. The new methods in this PR itself have head-only results and do not establish the optimization's benefit. ## Validation - `mise run lint:fix` — passed; formatter changes retained. - `mise run test` — passed. - All three benchmark tooling test scripts — 24 tests passed. - `mise run build -- -DskipITs=true` — passed. - Plain `mise run build` compiled the benchmark module, but Docker-backed integration tests ran despite `-DskipTests` and failed because no Docker environment was available. - JMH listing with each workflow pattern confirmed client_java-only selection and retained OpenMetrics/Prometheus exposition cases. - All four new benchmarks completed with GC profiling: 1 fork, 2 x 1s warmups, 3 x 1s measurements, JDK 25.0.3, `-Xms128m -Xmx256m`. This was a smoke test on a shared development host, not a controlled base/head performance comparison. Repeated lookup reported about 64 B/update; cached increments were near zero. - Generated the Markdown report from real smoke-run JSON and checked allocation output. - `git diff --check` — passed. --------- Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com> Co-authored-by: Jay DeLuca <jaydeluca4@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
StatefulMetric.labelValues(...)— the path everycounter.labelValues(...).inc()/histogram.labelValues(...).observe(...)goes through — calleddata.computeIfAbsent(key, l -> newDataPoint())on every invocation. The mapping functioncaptures
this(viametadata/labelNames), so it is not a cached singleton: a new lambdainstance is allocated on every call, including the overwhelmingly common case where the data point
already exists (the lambda argument is constructed before
computeIfAbsentruns, even though it isonly invoked on a miss).
This adds a
data.get(key)fast path that returns the existing data point without constructing thelambda:
Two secondary points folded in on the miss path:
computeIfAbsent, i.e. outside theConcurrentHashMapbin lock, and operates on the rawString[](noList.getindirection);computeIfAbsenton purpose —newDataPoint()has a side effect (a nativehistogram may schedule a reset task via
Scheduler.schedule), so at-most-once creation must bepreserved;
putIfAbsentwith a pre-built value would leak the loser's scheduled task on a race.Why
labelValues(...)is the hottest path in the library (every metric update). In a JMH benchmark of ahistogram-heavy workload, the
get()fast path cut record-path allocation by ~18% — exactly the16-byte captured lambda per observation.
Correctness
Behavior is unchanged (a null label value still throws on first use, since a null-containing key is
never inserted and so always reaches the miss branch). Verified by the core tests, including
StatefulMetricTest.This pull request and its description were written by Isaac.